feat(rpc): cap subscriptions per websocket connection - #4023

Claude finished @NazariiDenha's task in 6m 18s —— View job

Review complete

• Gather context (diff, changed files)
• Review jsonrpc/websocket.go changes (subscription cap logic)
• Review jsonrpc/server.go changes
• Review node/http.go, node/node.go, cmd/juno/juno.go (config plumbing)
• Review rpc/v8,v9,v10/subscriptions.go changes
• Review tests
• Post inline comments for issues found
• Post summary

Summary: The core mechanism is sound — TryAcquireSubscription/ReleaseSubscription use a correct lock-free CAS loop, every subscription entry point (SubscribeNewHeads, events, receipts, status, transactions across v8/v9/v10) funnels through the single h.subscribe() gate, and the slot is always released via defer inside the subscription goroutine with no early-return path between acquiring the slot and starting that goroutine (so no leak on onStart errors, panics, or cancellation). HandleReadWriter's new Transport interface only has one production caller (websocket.go), so the HTTP path is unaffected. Tests cover per-connection isolation, interleaved connections, and the refused-call-doesn't-leak-a-slot case.

Important:

• cmd/juno/juno.go (usage text around line 286): --rpc-max-ws-connections=0 does not disable the limit like every other rpc-max-* flag in this file — it makes semaphore.NewWeighted(0) reject every websocket connection (after a 5s hang) via acquireConnSlot. This is a footgun for an operator following the established "0 disables the limit" convention elsewhere in the same file. See inline comment for a suggested fix.

Nit:

• rpcMaxSubscriptionsUsage doesn't document that 0 means "unbounded" (which the code does support, unlike the connections flag above) — worth a one-line mention for consistency/clarity.

No correctness bugs, races, or resource leaks found otherwise. I was unable to execute go test/go vet in this sandboxed environment (commands required approval that wasn't grantable here), so verification was done by static reading of the diff and cross-referencing call sites; the reviewer may want to run the existing test suite (go test ./jsonrpc/... ./rpc/... ./node/... ./cmd/juno/...) to confirm.
· branch feat/ws-subscription-limits

Codecov Report

❌ Patch coverage is 93.87755% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.21%. Comparing base (f137027) to head (3d52998).

@@                 Coverage Diff                  @@
##           feat/ws-rpc-gate    #4023      +/-   ##
====================================================
- Coverage             79.22%   79.21%   -0.02%     
====================================================
  Files                   465      465              
  Lines                 35815    35854      +39     
====================================================
+ Hits                  28374    28401      +27     
- Misses                 7432     7444      +12     
  Partials                  9        9              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Claude finished @NazariiDenha's task in 4m 53s —— View job

Review complete

• Gather context (diff vs origin/feat/ws-rpc-gate, changed files)
• Review jsonrpc/websocket.go / jsonrpc/server.go changes (subscription + connection cap logic)
• Review node/http.go, node/node.go, cmd/juno/juno.go (config plumbing)
• Review rpc/v8,v9,v10/subscriptions.go changes
• Review tests
• Post inline comments for issues found
• Post summary

Summary: This update fixes both items from my previous pass — the PR is clean now, no inline comments.

• The --rpc-max-ws-connections=0 footgun is fixed: node/http.go (makeRPCOverWebsocket) now only builds the semaphore.Weighted when maxConns > 0, and Websocket.WithConnLimiter(nil) / acquireConnSlot treat a nil semaphore as unbounded, so 0 now means "no limit" consistent with every other rpc-max-* flag.
• rpcMaxWSConnectionsUsage and rpcMaxSubscriptionsUsage in cmd/juno/juno.go now both explicitly document the 0 = unbounded behavior.

I re-verified the core mechanism on the current diff: TryAcquireSubscription/ReleaseSubscription (jsonrpc/websocket.go) use a correct CAS loop, all v8/v9/v10 subscribe() entry points acquire a slot before any other fallible work and release it via defer inside the subscription goroutine, and Unsubscribe calls cancel() + wg.Wait() so the slot is guaranteed free by the time it returns (confirmed against rpc/v10/subscriptions_test.go's new TestSubscribeRespectsConnectionLimit, which exercises exactly this: refused call doesn't take a slot, slot frees after Unsubscribe, connection can resubscribe after). jsonrpc/websocket_test.go's new TestWebsocketSubscriptionSlotsArePerConnection confirms per-connection isolation over real websocket connections. go vet ./jsonrpc/... ./rpc/v8/... ./rpc/v9/... ./rpc/v10/... ./rpc/rpccore/... is clean.

Nits (non-blocking):

• rpc/v10/subscription_heads.go and subscription_events.go call resolveBlockRange (a bcReader.Height()/DB read) before h.subscribe() checks the subscription cap, so a connection already at its limit still pays for that read on every rejected SubscribeNewHeads/SubscribeEvents call. Reordering to check the cap first would avoid the wasted work, though it's not a correctness issue.
• rpc/v8/subscriptions_test.go and rpc/v9/subscriptions_test.go's fakeConn.TryAcquireSubscription always returns true, so unlike v10 there's no test exercising the ErrTooManySubscriptions path for those versions (matches Codecov's flagged missing lines in rpc/v8/subscriptions.go and rpc/v9/subscriptions.go). Not required since the logic is identical to v10's already-tested path, but a small coverage gap.

I couldn't run go test in this sandboxed environment (blocked by tool approval), so this was verified by static review plus go vet/go build where possible — worth confirming the full suite (go test ./jsonrpc/... ./rpc/... ./node/... ./cmd/juno/...) passes in CI.
· branch feat/ws-subscription-limits

Claude finished @NazariiDenha's task in 4m 12s —— View job

Review complete

• Gather context (diff vs origin/feat/ws-rpc-gate)
• Review jsonrpc/websocket.go / jsonrpc/server.go (subscription + connection cap logic)
• Review node/http.go, node/node.go, cmd/juno/juno.go (config plumbing)
• Review rpc/v8,v9,v10/subscriptions.go
• Review tests
• Post summary

Summary: No new changes since the last two review passes on this PR — the diff against origin/feat/ws-rpc-gate is identical to what was already reviewed and confirmed fixed. Re-verified the current code directly (not just prior comments):

• TryAcquireSubscription/ReleaseSubscription (jsonrpc/websocket.go:261-275) is a correct lock-free CAS loop; maxSubscriptions <= 0 is treated as unbounded.
• Every v8/v9/v10 subscribe() acquires the slot first (if !wsConn.TryAcquireSubscription() { return "", rpccore.ErrTooManySubscriptions }) and releases it via defer wsConn.ReleaseSubscription() inside the subscription goroutine — no leak path on onStart errors, panics, or cancellation.
• node/http.go's makeRPCOverWebsocket only builds the connection semaphore.Weighted when maxConns > 0, and acquireConnSlot/WithConnLimiter(nil) treat a nil semaphore as unbounded — the previously-flagged --rpc-max-ws-connections=0 footgun is fixed and stays fixed.
• rpcMaxWSConnectionsUsage and rpcMaxSubscriptionsUsage in cmd/juno/juno.go both document "0 disables the limit" now.
• go vet ./jsonrpc/... ./rpc/v8/... ./rpc/v9/... ./rpc/v10/... ./rpc/rpccore/... ./node/... ./cmd/juno/... is clean.

No blockers or new issues found. No inline comments needed.

Nits (still open, non-blocking, carried over from the prior review):

• rpc/v10/subscription_heads.go:27 and subscription_events.go:80 still call resolveBlockRange (a DB read) before h.subscribe() checks the cap, so a connection already at its limit pays for that read on every rejected call.
• rpc/v8/subscriptions_test.go and rpc/v9/subscriptions_test.go fakes still hardcode TryAcquireSubscription() bool { return true }, so unlike v10 (limitedConn / TestSubscribeRespectsConnectionLimit) there's no test exercising ErrTooManySubscriptions for those versions.

I could not run go test in this sandboxed environment (blocked by tool approval), so this was verified via static review plus a clean go vet — the reviewer may want to confirm the full suite passes in CI.
· branch feat/ws-subscription-limits

PR Reviewer Guide 🔍

Here are some key observations to aid the review process: